-
Notifications
You must be signed in to change notification settings - Fork 2.1k
CCO-453: Add CloudCredential cap for v415 #46551
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@fxierh: This pull request references CCO-453 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
Skipping CI for Draft Pull Request. |
|
@fxierh: This pull request references CCO-453 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@fxierh, Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
/pj-rehearse periodic-ci-openshift-openshift-tests-private-release-4.15-multi-nightly-gcp-ipi-baselinecaps-none-arm-f7 periodic-ci-openshift-openshift-tests-private-release-4.15-multi-nightly-azure-ipi-baselinecaps-vset-additionalcap-arm-f14 periodic-ci-openshift-openshift-tests-private-release-4.15-amd64-nightly-4.15-upgrade-from-stable-4.14-aws-upi-baselinecaps-none-f28 |
|
/pj-rehearse periodic-ci-openshift-openshift-tests-private-release-4.15-multi-nightly-gcp-ipi-baselinecaps-none-arm-f7 periodic-ci-openshift-openshift-tests-private-release-4.15-multi-nightly-azure-ipi-baselinecaps-vset-additionalcap-arm-f14 periodic-ci-openshift-openshift-tests-private-release-4.15-amd64-nightly-4.15-upgrade-from-stable-4.14-aws-upi-baselinecaps-none-f28 |
|
/cc @jianlinliu @jinyunma Would you please take a look ? Thank you. |
|
The upgrade change looks good to me. Thanks |
| fi | ||
|
|
||
| # For OCP 4.15, CCO is required for non-BareMetal platforms | ||
| if [[ "$ocp_major_version" == "4" ]] && [[ "$ocp_minor_version" == "15" ]] && [[ "${selected_capability}" == "CloudCredential" ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we don't need to check ocp version here, since cap CloudCredential is only in capset v4.14+ and vCurrent on 4.15+.
If there is any change in coming version (e.g 4.16), we can update the logic accordingly then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
| fi | ||
|
|
||
| # For OCP 4.15, CCO is required for non-BareMetal platforms | ||
| if [[ "$ocp_major_version" == "4" ]] && [[ "$ocp_minor_version" == "15" ]] && [[ "${selected_capability}" == "CloudCredential" ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| if [[ "$ocp_major_version" == "4" ]] && [[ "$ocp_minor_version" == "15" ]] && [[ "${selected_capability}" == "CloudCredential" ]]; then | |
| if [[ "${selected_capability}" == "CloudCredential" ]]; then |
On the line 66, only 4.15 has the possibility to select CloudCredential, so I think do not need to check version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pamoedom @aleskandro Do you know how to judge if the job is running on baremetal platform? Or shall we ignore the platform checking for now, because there is no baseline=none baremetal install yet?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, baremetal-like installations have either Platform: None (upi-like) or Platform: Baremetal (ipi-like) set in the install config. If day2, the same information should be available in the infrastructure/cluster object
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jianlinliu @aleskandro
I guess I should read install-config.platform and allow CCO to be disabled only if platform is baremetal or none, right ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jianlinliu CLUSTER_TYPE appears to take many possible values. It is possible to use it (with some regex matches for example) but I guess it's not going to be straightforward.
https://github.com/openshift/ci-tools/blob/21cd5e74813c87fa5e72e6f95aa0407b12014918/pkg/api/types.go#L1322
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, because we only focus on baremetal ones, we do not have to match all the supported cluster type via regex. I saw some other step is using something like the following to match baremetal cluster profile,
"${CLUSTER_TYPE}" =~ ^packet.*$|^equinix.*$
I am not if we have some more other baremetal cluster types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current cluster profile name has equinix in its name. I do not remember if CLUSTER TYPE comes from that and I'm afk today. I wonder if vsphere and other similar scenarios should be excluded too though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed. Currently BM profiles have CLUSTER_TYPE prefixed with packet or equinix in ci-tools.
Using this now:
"${CLUSTER_TYPE}" =~ ^packet.*$|^equinix.*$
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For 4.15 the disablement of CCO is supported for BM only.
Whether/when to support other platforms is still under discussion.
|
/pj-rehearse periodic-ci-openshift-openshift-tests-private-release-4.15-arm64-nightly-aws-ipi-disconnected-sts-basecap-none-f28 |
|
/pj-rehearse periodic-ci-openshift-openshift-tests-private-release-4.15-arm64-nightly-aws-ipi-disconnected-sts-basecap-none-f28 |
|
/pj-rehearse periodic-ci-openshift-openshift-tests-private-release-4.15-arm64-nightly-aws-ipi-disconnected-sts-basecap-none-f28 periodic-ci-openshift-openshift-tests-private-release-4.15-multi-nightly-gcp-ipi-baselinecaps-none-arm-f7 periodic-ci-openshift-openshift-tests-private-release-4.15-multi-nightly-azure-ipi-baselinecaps-vset-additionalcap-arm-f14 periodic-ci-openshift-openshift-tests-private-release-4.15-amd64-nightly-4.15-upgrade-from-stable-4.14-aws-upi-baselinecaps-none-f28 |
|
@fxierh: job(s): periodic-ci-openshift-openshift-tests-private-release-4.15-multi-nightly-gcp-ipi-baselinecaps-none-arm-f7 either don't exist or were not found to be affected, and cannot be rehearsed |
|
/pj-rehearse periodic-ci-openshift-openshift-tests-private-release-4.15-multi-nightly-gcp-ipi-baselinecaps-none-arm-f7 |
|
@fxierh: job(s): periodic-ci-openshift-openshift-tests-private-release-4.15-multi-nightly-gcp-ipi-baselinecaps-none-arm-f7 either don't exist or were not found to be affected, and cannot be rehearsed |
|
[REHEARSALNOTIFIER]
A total of 2775 jobs have been affected by this change. The above listing is non-exhaustive and limited to 25 jobs. A full list of affected jobs can be found here Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
/pj-rehearse periodic-ci-openshift-openshift-tests-private-release-4.15-arm64-nightly-aws-ipi-disconnected-sts-basecap-none-f28 |
|
@fxierh: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
periodic-ci-openshift-openshift-tests-private-release-4.15-arm64-nightly-aws-ipi-disconnected-sts-basecap-none-f28 failed due to an I guess this should not block this PR. |
|
/lgtm |
|
cc @jinyunma to double confirm. |
|
/lgtm |
|
/cc @liangxia |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fxierh, jianlinliu, jinyunma, liangxia The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/pj-rehearse ack |
|
/retest-required |
openshift/oc#1622 and its cherry-pick landed. Relevant CI jobs should be okay soon. cc. @jianlinliu @jinyunma |
The CloudCredential cap was introduced in OCP 4.15, see https://issues.redhat.com/browse/OCPEDGE-69.
One thing to note:
The disablement of CCO is supported on BareMetal platforms only in OCP 4.15, so non-BareMetal profiles must have CCO enabled.